transport: use go 1.19 atomic.Pointer instead of atomic.Value #284
Conversation
Kulezi
force-pushed
the
pp/go-1.19-patch
branch
3 times, most recently
from
5fa174f
to
bd3757b
Compare
Kulezi
changed the title
Kulezi
force-pushed
the
pp/go-1.19-patch
branch
4 times, most recently
from
9235bba
to
15058fa
Compare
transport/cluster.go
Outdated
| @@ -443,7 +443,7 @@ func parseTokensFromRow(n *Node, r frame.Row, ring *Ring) error { | |||
| } | |||
|
|
|||
| func (c *Cluster) Topology() *topology { | |||
There was a problem hiding this comment.
Is Topology() method still necessary? It is an exported method returning non-exported type, so I wonder if we could just remove it in favor of using c.topology.Load() directly.
There was a problem hiding this comment.
Topology() is used in session to get all nodes to prepare a query on them. Nodes are the only field of topology that is currently exported and used outside of transport, so I suppose Topology() could be removed and replaced with just func (*Cluster) Nodes().
The other thing is that topology is passed inside QueryInfo to host selection policies, but same as all fields of QueryInfo, it is unexported. It likely should be accessible outside of transport, opened #287 about it, it may influence if topology will be exported.
The point of Topology() and setTopology() cluster methods was to give some sort of type safety as topology was an atomic.Value that can hold any type. As topology is now an atomic.Pointer they are no longer needed. Session sometimes needs to perform a query on all nodes of the cluster, to allow that cluster now has an exported Nodes() method.
Kulezi
force-pushed
the
pp/go-1.19-patch
branch
from
15058fa
to
f757a77
Compare
Currently transport package atomically replaces topology and connections with new ones. This is done using atomic.Value that hold topology and conn pointers, this is not type-safe and requires type assertions every time the driver needs the values hidden in atomic.Value.
Go 1.19 introduces a generic atomic.Pointer[T], that is a more suitable replacement, adding type-safety and removing the type assertions,
This PR replaces mentioned atomic.Values with the new generic atomic pointer type.
Fixes #269